Adds shake command #2259
Adds shake command #2259
Conversation
ivanacostarubio
force-pushed
the
feature/shake
branch
from
9050e69 to
3a95df2
Compare
| if (exitCode != 0) { | ||
| val errorStream = process.errorStream.bufferedReader().readText() | ||
| logger.error("Failed to execute shake gesture. Exit code: $exitCode, Error: $errorStream") | ||
| throw IOException("Failed to execute shake gesture on simulator. Exit code: $exitCode") |
There was a problem hiding this comment.
The catch block below will re-throw this with this message wrapped in the same message again:
"Failed to execute shake gesture on simulator: Failed to execute shake gesture on simulator. Exit code: $exitCode"
There was a problem hiding this comment.
Thanks for the review. 68212e0
| logger.error("Failed to execute shake gesture. Exit code: $exitCode, Error: $errorStream") | ||
| } | ||
|
|
||
| logger.trace("Successfully executed shake gesture") |
There was a problem hiding this comment.
Now if exitCode != 0 you will log the error, but don't do anything about it and you'll log this 'Successful' message and return normally.
Sorry I wasn't more clear in the earlier review. I'd suggest bringing the previous throw back, so the function exits properly on error. But something else needs to change to avoid the double message.
I thought of two options.
- cut down the message:
throw IOException("Exit code: $exitCode")and the outer catch/throw will use that as the error message. - Move the code that checks the exitCode outside the try block (everything from 'if' on 198 to 'logger.trace' on 203). That way so the
throw IOExceptioncan throw directly to the calling function. That means the calling function will get the exception as IOException instead of XCUITestServerError.UnknownFailure.
There was a problem hiding this comment.
While I was writing the other comment about the deleted 'throw' I decided to look at the rest of the XCTestDriverClient file for other examples of throwing exceptions, and I noticed that this function is different from all the others that call executeJsonRequest. Looks like you need to move all this code to a new Handler file in /Maestro/maestro-ios-xctest-runner/maestro-driver-iosUITests/Routes/Handlers/ (and add a new case to the Rouse enum in XCTestHTTPServer.swift and handle that case in createRouteHandler in RouteHandlerFactory.swift).
Then instead of throwing exceptions you'll want to return HTTP errors.
Of course, this is all from looking at this file for a few minutes, I'm not one of the original developers, so we'll want one of them to chime in as well.
Proposed changes
It implements the shake command. It closes #1164
AI
copilot:summary
Testing
I was able to test this using my local iOS simulator. However, I'm unsure why it's not working on the Android emulator. I can see the shake happening when I use the terminal to run the adb commands.
Issues fixed